Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maccel scaledowncurve+roundingcarry #6

Merged
merged 31 commits into from
Apr 9, 2024

Conversation

Wimads
Copy link
Collaborator

@Wimads Wimads commented Mar 5, 2024

Changing the acceleration math to scale down from default DPI instead of scaling up as before.
Adding a rounding carry to make it smooth at low speeds (credit: @ankostis )

  • Limit variable now is a lower limit instead of upper limit, as factor of Default DPI.
  • Upper limit is now fixed to 1.1 * default DPI
  • User is expected to leave DPI setting to their usual non-accelerated DPI, rather than a low DPI as before.

@burkfers burkfers force-pushed the maccel-scaledowncurve+roundingcarry branch from 642e264 to 97bbf32 Compare March 6, 2024 07:43
maccel/maccel.c Outdated Show resolved Hide resolved
maccel/maccel.c Outdated Show resolved Hide resolved
maccel/maccel.c Outdated Show resolved Hide resolved
@burkfers
Copy link
Owner

burkfers commented Mar 7, 2024

This requires a documentation change, including breaking change instructions.

@burkfers
Copy link
Owner

burkfers commented Mar 7, 2024

I've pushed an attempt at documenting the changes.
@Wimads please let me know what you think.

@burkfers
Copy link
Owner

burkfers commented Mar 7, 2024

@Wimads Can you update the desmos image and link for the new formula?

@burkfers burkfers force-pushed the maccel-scaledowncurve+roundingcarry branch from fa7de95 to efa9af0 Compare March 8, 2024 08:38
@burkfers
Copy link
Owner

burkfers commented Mar 8, 2024

Note for self, from Discord:

Also, @burkfers, I think the ranges/steps for keycodes and via might need to be adjusted. Something like this would do I think:

Variable:    |  Range:      |  Step:
-------------+--------------+--------
TAKEOFF      |  0.50 - 5.00 |  0.01
GROWTH_RATE  |  0.01 - 5.00 |  0.01
OFFSET       |  0.0  - 15.0 |  0.1   
LIMIT        |  0.00 - 1.00 |  0.01

@burkfers burkfers force-pushed the maccel-scaledowncurve+roundingcarry branch from 7f7ab25 to d79ecdb Compare March 10, 2024 18:16
@Wimads
Copy link
Collaborator Author

Wimads commented Mar 10, 2024

Can you explain this part @finrod09 ?
https://github.com/finrod09/qmk_userspace_features/blob/5cba04202c5d79945cf24fb32b361220446971cd/maccel/readme.md?plain=1#L118-L124

Throttling was necessary for myself to get my dragscroll implementation to work smoothly. Don't know what you mean with the linearity issue you describe here?

Also the line above (116) is duplication, I moved that to the bit above the preceding code block. Guess that got back in from a merge conflict.

@burkfers
Copy link
Owner

burkfers commented Mar 10, 2024

Guess that got back in from a merge conflict.

Correct, I botched rebasing onto the spelling fix in maccel-dev. No content changes were intended.

Since it is likely #8 will merge before this, I will get another chance at rebasing correctly.

Edit: This was a result of this PR's history being convoluted and already containing merges that have to be manually resolved. We have let this PR linger too long and it has become bloated. A lesson for all involved, hopefully we'll do better next time :)

@Wimads
Copy link
Collaborator Author

Wimads commented Mar 10, 2024

Guess that got back in from a merge conflict.

Correct, I botched rebasing onto the spelling fix in maccel-dev. No content changes were intended.

Since it is likely #8 will merge before this, I will get another chance at rebasing correctly.

Edit: This was a result of this PR's history being convoluted and already containing merges that have to be manually resolved. We have let this PR linger too long and it has become bloated. A lesson for all involved, hopefully we'll do better next time :)

Yeah I'm getting a lot of merge conflicts lately on this branch when syncing to my machine... Does this have to do with the spell check? All commits I did lately also got a spell check error.

@Wimads
Copy link
Collaborator Author

Wimads commented Mar 10, 2024

But what about that throttling part in the readme? Is that also something that got back in accidentally over a merge, or was that intentional?

@burkfers burkfers force-pushed the maccel-scaledowncurve+roundingcarry branch from c183e9f to c1c62c7 Compare March 12, 2024 10:27
@burkfers burkfers force-pushed the maccel-scaledowncurve+roundingcarry branch from be1a705 to ef443f6 Compare March 12, 2024 14:16
@burkfers burkfers self-requested a review March 12, 2024 14:44
maccel/maccel.c Show resolved Hide resolved
maccel/readme.md Outdated Show resolved Hide resolved
@burkfers burkfers merged commit 8376941 into maccel-dev Apr 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants